Skip to content

Use obligation ctxt instead of dyn TraitEngine#104509

Merged
bors merged 3 commits intorust-lang:masterfrom
spastorino:use-obligation-ctxt
Nov 23, 2022
Merged

Use obligation ctxt instead of dyn TraitEngine#104509
bors merged 3 commits intorust-lang:masterfrom
spastorino:use-obligation-ctxt

Conversation

@spastorino
Copy link
Member

@spastorino spastorino commented Nov 16, 2022 •
edited
Loading

r? @lcnr

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 16, 2022
compiler-errors
compiler-errors reviewed Nov 16, 2022
View reviewed changes
compiler/rustc_traits/src/implied_outlives_bounds.rs Outdated
Comment on lines +117 to 121
Copy link
Contributor

@compiler-errors compiler-errors Nov 16, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does calling select_all_or_error here do to the select_* calls that happen in make_query_response, since we're inside of a canonical query here?

Copy link
Contributor

@compiler-errors compiler-errors Nov 16, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Maybe it does nothing -- just curious, since we're passing down the ObligationCtxt from the canonical query instead of creating a trait engine inline, it might change behavior -- but maybe not!)

Copy link
Contributor

@lcnr lcnr Nov 17, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is necessary to constrain inference variables which we use further down when computing the implied bounds. Can you add a comment mentioning that here?

@compiler-errors
Copy link
Contributor

compiler-errors commented Nov 16, 2022

(nit: also the commits for fully_solve_obligation, fully_solve_obligation, and fully_solve_obligation could be squashed since they're right next to each other and also almost identical -- or they could be reworked so they all call fully_solve_obligations 😃 )

@spastorino
Copy link
Member Author

spastorino commented Nov 16, 2022

Yes, I'm going to squash them all. Right now with the cellphone but will do ASAP and also what you're suggesting.

@compiler-errors
Copy link
Contributor

compiler-errors commented Nov 17, 2022

No rush anyways, thanks 👍

@lcnr
Copy link
Contributor

lcnr commented Nov 17, 2022

r=me after squashing some commits and adding a comment to compiler/rustc_traits/src/implied_outlives_bounds.rs

@spastorino
Copy link
Member Author

spastorino commented Nov 17, 2022

This is now ready!

@lcnr
Copy link
Contributor

lcnr commented Nov 17, 2022

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Nov 17, 2022

📌 Commit 9864a63 has been approved by lcnr

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Nov 17, 2022

🌲 The tree is currently closed for pull requests below priority 1. This pull request will be tested once the tree is reopened.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 17, 2022
@spastorino
Copy link
Member Author

spastorino commented Nov 22, 2022

Unsure what happened with this PR but I think it needs to be re-approved.

@bors r=lcnr

@bors
Copy link
Collaborator

bors commented Nov 22, 2022

📌 Commit 9864a63 has been approved by lcnr

It is now in the queue for this repository.

Manishearth added a commit to Manishearth/rust that referenced this pull request Nov 22, 2022
@Manishearth
Rollup merge of rust-lang#104509 - spastorino:use-obligation-ctxt, r=…
bbe1843 
…lcnr

Use obligation ctxt instead of dyn TraitEngine

r? `@lcnr`
@Manishearth Manishearth mentioned this pull request Nov 22, 2022
Closed
@compiler-errors
Copy link
Contributor

compiler-errors commented Nov 22, 2022 •
edited
Loading

This needs re-bless, cc #104738

@Manishearth
Copy link
Member

Manishearth commented Nov 22, 2022

@bors r-

#104738 (comment)

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 22, 2022
@spastorino
Copy link
Member Author

spastorino commented Nov 23, 2022

@bors r=lcnr

@bors
Copy link
Collaborator

bors commented Nov 23, 2022

📌 Commit 859b147 has been approved by lcnr

It is now in the queue for this repository.

@bors bors removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Nov 23, 2022
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Nov 23, 2022
@Dylan-DPC Dylan-DPC mentioned this pull request Nov 23, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 23, 2022
@bors
Auto merge of rust-lang#104776 - Dylan-DPC:rollup-rf4c2u0, r=Dylan-DPC
d121aa3 
Rollup of 6 pull requests

Successful merges:

 - rust-lang#104269 (Fix hang in where-clause suggestion with `predicate_can_apply`)
 - rust-lang#104286 (copy doc output files by format)
 - rust-lang#104509 (Use obligation ctxt instead of dyn TraitEngine)
 - rust-lang#104721 (Remove more `ref` patterns from the compiler)
 - rust-lang#104744 (rustdoc: give struct fields CSS `display: block`)
 - rust-lang#104751 (Fix an ICE parsing a malformed attribute.)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 0a79138 into rust-lang:master Nov 23, 2022
@rustbot rustbot added this to the 1.67.0 milestone Nov 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lcnr lcnr lcnr left review comments

+1 more reviewer

@compiler-errors compiler-errors compiler-errors left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@lcnr lcnr

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.67.0

Development

Successfully merging this pull request may close these issues.

6 participants

@spastorino @compiler-errors @lcnr @bors @Manishearth @rustbot